feat(nodes): rich node cards with rename, forget, and tray cleanup - #324
Conversation
Tray-app changes covering four areas of how the operator manages paired
nodes:
1. Tray flyout — the "Devices" section now shows only currently-connected
nodes. Stale paired-but-offline entries no longer accumulate in the
right-click menu; they remain accessible on the full Nodes page.
2. Gateway model + parser — `GatewayNodeInfo` now carries the rest of the
`NodeListNode` schema the gateway already sends: `Version`,
`CoreVersion`, `UiVersion`, `ClientId`, `ClientMode`, `DeviceFamily`,
`ModelIdentifier`, `RemoteIp`, `PathEnv`, `ConnectedAt`, `ApprovedAt`,
`LastSeenReason`, `IsPaired`, `HasExplicitDisplayName`, plus the
`DisabledCommands` list. `ParseNodeList` now reads the production
`*Ms` timestamp wire names (`lastSeenAtMs`, `connectedAtMs`,
`approvedAtMs`); the legacy non-Ms fallbacks remain for mocks/tests.
`LastSeen` no longer falls back to `connectedAtMs` so the same value
doesn't appear twice in the UI.
3. New gateway client methods:
- `NodeRenameAsync` — awaits the gateway response via
`SendWizardRequestAsync` and returns a `NodeRenameResult` with
success/error so the UI can surface the actual server message.
- `NodePairRemoveAsync` — same pattern, returns a `NodeForgetResult`.
Using `SendWizardRequestAsync` (rather than the fire-and-forget
`TrySendTrackedRequestAsync`) means scope rejections, missing
nodeIds, and other application-level failures are reported back to
the caller instead of being silently swallowed.
The `node.pair.resolved` broadcast handler now refreshes both the
pair-list and the node list so removed nodes disappear immediately.
4. NodesPage — full rewrite of the per-node card. Each node renders as
an `Expander` (online auto-expanded, offline collapsed) with a body
that shows the identity row, version line, hardware, network,
timestamps, capability tags (now actually populated), commands list
with disabled annotations, permissions grid, and an optional
collapsed PATH dump. Action footer at the bottom — separator + a
right-aligned [Rename] [Forget] pair — follows the Win11 Settings
pattern (Manage / Remove on Email account cards). Both actions open
`ContentDialog`s using the deferral pattern so failures stay inline
instead of silently closing.
Implementation notes worth highlighting:
- Click lambdas wrap their async work in try/catch so an unhandled
exception in a dialog flow can never become an `async void` crash.
- Rename TextBox pre-fills with the explicit display name only — never
with the parser's fallback id — so pressing Enter on an unnamed node
doesn't persist the id as the new display name.
- Layout uses `Grid` with star/auto columns where `TextWrapping` and
`TextTrimming` actually matter (header, identity row, label rows);
long ids/names ellipsize with a tooltip, long values wrap.
- `ModelFormatting.FormatAge` was made `public` and grew a clock-skew
guard plus a >30d absolute-date branch; both the header `DetailText`
and the body timestamps now share it, so a single timestamp can't
show as "1d ago" in one place and "36h ago" in another.
- 25 new resource keys per locale (en-us, fr-fr, nl-nl, zh-cn, zh-tw);
"Version", "Hardware", and "PATH" are registered as invariant.
Tests:
- `OpenClaw.Shared.Tests` adds 7 new parser tests covering the full
`NodeListNode` schema, legacy wire-name compatibility, minimal
payload defaults, and rename/forget input validation paths.
- `OpenClaw.Tray.Tests` mock implementation of `IOperatorGatewayClient`
updated for the new methods. Localization validation tests pass with
the three invariant exceptions registered.
Validation: `dotnet build` clean; `OpenClaw.Shared.Tests` 1547 passed,
`OpenClaw.Tray.Tests` 965 passed.
Known follow-up (out of scope for this PR, deserves its own change):
- Action buttons are not yet gated on `operator.pairing` scope.
Bootstrap-token sessions will see enabled buttons that fail with the
gateway's "missing scope" message inline — same as the existing pair
approve/reject buttons. A wider scope-aware UI pass should expose
granted scopes on `IOperatorGatewayClient` and gate everywhere.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Review feedback from adversarial PR #324 passI reviewed this with a critical eye against the tray/gateway interaction paths, and also cross-checked it with a two-model review. Overall this looks solid: the PR is clean/mergeable, CI is green, the gateway parser changes line up with the richer I do recommend addressing one small WinUI dialog reentrancy issue before merge. The remaining items are follow-up/polish level. Should fix before merge: page-wide ContentDialog guard
That prevents double-clicking Rename/Forget for one node, but it does not prevent a second dialog for a different node from being shown while the first dialog is still opening/open. WinUI only allows one Impact: a fast user path like Rename on node A, then Forget on node B can make the second Suggested fix: make the guard page-wide instead of per-node, or at least check the set count before adding the current node: if (_nodesWithDialogOpen.Count > 0) return;
if (!_nodesWithDialogOpen.Add(node.NodeId)) return;A clearer alternative would be a dedicated Follow-up / polish items
input.Loaded += (_, _) =>
{
input.Focus(FocusState.Programmatic);
input.SelectAll();
};
|
|
🤖 This is an automated response from Repo Assist. Solid PR — the rich Expander cards are a big UX improvement, and the 196 new tests for the gateway client changes are excellent. A few observations:
Rename pre-fill: The description correctly notes the rename Known follow-ups: The scope-aware gating note in the PR description is a clear and honest callout — the inline failure message from the gateway on scope rejection is acceptable for now. These are minor points on an otherwise clean contribution; the core implementation looks well thought-out.
|
Three issues raised in the PR review: 1. Page-wide ContentDialog reentrancy. WinUI 3 only allows a single ContentDialog per XamlRoot, so the per-node `_nodesWithDialogOpen` HashSet did not protect against Rename-on-A then Forget-on-B fast paths — the second `ShowAsync` would throw and be swallowed by the click-handler catch. Replaced with a page-wide `_dialogOpen` bool, matching the convention `SandboxPage._confirmDialogOpen` already uses. 2. Rename TextBox selection. `input.SelectAll()` ran before the box was attached to the visual tree, so the pre-filled name was never actually selected. Moved focus+select-all into the `Loaded` event so it fires after the dialog inserts the TextBox. 3. Raw `ex.Message` leaking into the UI. NodeRenameAsync and NodePairRemoveAsync used to return whatever the catch block saw, including internal transport/timeout exception text. Split the catch by type: `InvalidOperationException` (the gateway's ok=false ack) is still surfaced verbatim because the messages are actionable (e.g. "missing scope: operator.pairing"); every other exception now returns a null ErrorMessage so the dialog falls back to its localized generic string instead of showing internal text. Also removed the AccentButtonStyle on the Forget confirmation primary button. The destructive primary should not be styled as a call-to- action; Cancel remains the default focus. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Thanks for the careful read on both passes. Pushed Should-fix
Polish
Also dropped the Tracked for follow-up (not in this commit)
|
2 participants
Summary
Tray-app changes covering four areas of how the operator manages paired nodes:
Tray flyout — the Devices section now shows only currently-connected nodes. Stale paired-but-offline entries no longer accumulate in the right-click menu; they remain accessible on the full Nodes page.
Gateway model + parser —
GatewayNodeInfonow carries the rest of theNodeListNodeschema the gateway already sends (Version,CoreVersion,UiVersion,ClientId,ClientMode,DeviceFamily,ModelIdentifier,RemoteIp,PathEnv,ConnectedAt,ApprovedAt,LastSeenReason,IsPaired,HasExplicitDisplayName,DisabledCommands).ParseNodeListnow reads the production*Mstimestamp wire names; legacy non-Ms fallbacks remain for mocks/tests.LastSeenno longer falls back toconnectedAtMsso the same value never appears twice in the UI.New gateway client methods:
NodeRenameAsync→ returnsNodeRenameResult(success/error message)NodePairRemoveAsync→ returnsNodeForgetResult(success/error message)Both use
SendWizardRequestAsyncrather than the fire-and-forgetTrySendTrackedRequestAsync, so scope rejections, missing nodeIds, and other application-level failures are reported back to the caller instead of being silently swallowed. Thenode.pair.resolvedbroadcast handler now refreshes both the pair list and the node list.NodesPage — full rewrite of the per-node card. Each node renders as an
Expander(online auto-expanded, offline collapsed) with a body that shows the identity row, version line, hardware, network, timestamps, capability tags (now actually populated), commands list with disabled annotations, permissions grid, and an optional collapsed PATH dump. Action footer at the bottom — separator + a right-aligned Rename / Forget pair — follows the Win11 Settings pattern (Manage / Remove on Email account cards). Both actions openContentDialogs using the deferral pattern so failures stay inline instead of silently closing.Implementation notes
try/catchso an unhandled exception in a dialog flow can never become anasync voidcrash.TextBoxpre-fills with the explicit display name only — never with the parser's fallback id — so pressing Enter on an unnamed node doesn't persist the id as the new display name.Gridwith star/auto columns whereTextWrappingandTextTrimmingactually matter (header, identity row, label rows); long ids/names ellipsize with a tooltip, long values wrap.ModelFormatting.FormatAgewas madepublicand grew a clock-skew guard plus a >30d → absolute date branch; both the headerDetailTextand the body timestamps now share it, so a single timestamp can't show as "1d ago" in one place and "36h ago" in another.Version,Hardware, andPATHare registered as invariant.Tests
OpenClaw.Shared.Tests— +11 tests covering the fullNodeListNodeschema, legacy wire-name compatibility, minimal-payload defaults, and rename/forget input validation paths. Total: 1547 passed, 28 skipped, 0 failed.OpenClaw.Tray.Tests— mock implementation ofIOperatorGatewayClientupdated for the new methods. Localization validation tests pass with the three invariant exceptions registered. Total: 965 passed, 0 failed.dotnet buildclean.Screenshots
The Devices section in the tray flyout no longer lists offline duplicates. Each Nodes page card now shows a full Expander with rename/forget at the bottom. (Manual smoke tested against a live gateway.)
Known follow-ups (out of scope for this PR)
operator.pairing. Bootstrap-token sessions will see enabled buttons that fail with the gateway's "missing scope" message inline — same as the existing pair approve/reject buttons. A wider scope-aware UI pass should expose granted scopes onIOperatorGatewayClientand gate everywhere consistently.StackPanel(noWrapPanelin WinUI 3 base). Real but low-impact; defer until a paired node with many caps surfaces overflow.Validation checklist
./build.ps1cleandotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csprojdotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csprojCo-authored-by: Copilot 223556219+Copilot@users.noreply.github.com